Skip to content

feat: Send incremental state as checkpoints after the resources it covers - #2632

Merged
kodiakhq[bot] merged 22 commits into
mainfrom
claude/incremental-syncs-design-08b5fc
Oct 19, 2026
Merged

kodiakhq[bot] merged 22 commits into
mainfrom
claude/incremental-syncs-design-08b5fc

Conversation

@disq

@disq disq commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Summary

Adds source checkpoints and destination flushes, so the CLI can persist state only after its resources commit.

Sources opt in with state.ClientOptions{Checkpoints: res} and NewConnectedClientWithOptions, destinations with plugin.WithFlushSupport(). state.NewClientWithOptions returns an error when Checkpoints is set, as it cannot tell whether the CLI persists checkpoints.
With StrategyShuffleQueue, checkpointed state is held until the scheduler's Sync completes.
New write API: message.WriteFlush, message.FailedTablesError and message.FailedTables, writers.WithFailedTables for custom writers, and SkipFlush in the writer test suite.
Behaviour change: with checkpoints on, a failed batch write is reported on the next flush, with the tables it failed for, so the CLI stops saving state for that sync instead of only logging the failure.
After a cancelled sync, including a quota stop, Flush returns state.ErrSyncAborted for keys set in table resolvers and does not save them, as the scheduler may have dropped their rows.
scheduler.Sync now returns the cancel cause, not nil, when the context is cancelled while the batcher closes.
Also fixes NewConnectedClientWithOptions with a zero MaxMsgSizeInBytes, which set a 0-byte gRPC message limit.
Uses the protocol from plugin-pb-go v1.28.2.

@erezrokah

Copy link
Copy Markdown
Member

Found two issues in the streaming flush path:

  • Flush can report success after startWorker rejects an insert, allowing a checkpoint to advance past unwritten rows. Dispatch errors should propagate to the flush result.
  • Cancellation during commit abandons the unbuffered acknowledgement channel, leaving the worker and Close blocked indefinitely. A buffered or cancellation-aware acknowledgement would avoid this.

Both reproduced in focused tests.

@disq

disq commented Oct 14, 2026 •

Copy link
Copy Markdown
Member Author

@erezrokah Thanks, both reproduced and fixed in ca3497e:

  • A message startWorker rejects now fails every later flush in that Write call, so a checkpoint cannot pass unwritten rows (TestFlushMessageReportsRejectedMessages).
  • The commit acknowledgement channel is buffered, so a cancelled flush no longer blocks the worker or Close (TestFlushMessageCancelledDuringCommit).

disq added 2 commits October 14, 2026 12:50
- Skip a checkpoint when the sync is cancelled while flushing the batcher
- Make a key changed by several resolvers wait for all of their resources
- Try every checkpoint group in Flush, unscoped keys first, and export ErrSyncAborted
- Coalesce state held until a shuffle-queue sync completes
- Let clients turn flush support off per spec, and document that a flush ends open batches
- Only honour ClientOptions.Checkpoints in NewConnectedClientWithOptions
- Stop WriteStream when the plugin stops writing, and reject empty write messages
- Test that a flush is acknowledged only after it commits, and only if it succeeds
@erezrokah

Copy link
Copy Markdown
Member

The original issues are fixed. I reproduced two remaining problems:

  • A later flush of a shared cursor can overtake an earlier resolver’s unfinished rows because flush clears its pending dependencies.
  • Shuffle-queue can combine valid checkpoints into one oversized message, causing the sync to fail.

Could we preserve shared-key dependencies across flushes and split coalesced checkpoints to respect the message limit?

- A checkpoint of a key waits for the previous checkpoint of that key to be sent, so a later flush cannot overtake rows an earlier resolver has not sent yet
- Checkpoints merged during a shuffle-queue sync are marked sent once the merged checkpoint is sent
- Oversized checkpoints are split across several messages, like inserts
@disq

disq commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@erezrokah Both fixed in 363038e:

  • The state client keeps each key's last checkpoint until it is sent, and the next checkpoint of that key waits for it, so a later flush of a shared cursor can no longer overtake an earlier resolver's unsent rows (TestStateClientCheckpointWaitsForPreviousCheckpointOfKey). Checkpoints merged during a shuffle-queue sync count as sent once the merged one is sent.
  • The server now splits an oversized checkpoint across several messages, the same way it splits inserts, so a merged shuffle-queue checkpoint or a single large Flush stays within the message limit (TestSendRecordSplitsOversizedCheckpoint).

@erezrokah

Copy link
Copy Markdown
Member

All four earlier issues are fixed. I found one regression: dependency barriers bypass shuffle-queue coalescing, retaining every intermediate cursor record until sync completion.

The same state-client test produces 1 checkpoint for 100 flushes on the previous revision, but 100 checkpoints now. Could we preserve coalescing when tracking prior-checkpoint dependencies and cover the actual SetKey/Flush path in a regression test?

@disq

disq commented Oct 14, 2026

Copy link
Copy Markdown
Member Author

@erezrokah Fixed in b2202a5: a scope can now say a checkpoint passed to it is never sent before one it already holds, and the shuffle-queue scope does so for checkpoints merged into its held state. A repeated flush of a key in a shuffle-queue sync then skips the dependency barrier and is merged again. TestShuffleQueueCoalescesFlushedCheckpoints covers the SetKey/Flush path: 100 flushes produce one checkpoint, sent after every row.

disq added 7 commits October 14, 2026 19:09
- Track the scopes of a key only with checkpoints, as a set, and collapse the scopes of ended table resolvers into one, so SetKey stays O(1) and Flush linear
- A same-value SetKey of a pending key also waits for the caller's resources
- Abort checkpoints the scheduler drops, and the checkpoints waiting for them, so a later Flush of their keys returns ErrSyncAborted and keeps them pending
- Route a checkpoint changed outside resolvers through the scope holding what it waits for, so shuffle-queue keeps merging
- Do not report flush support for clients without a connection
- Name the table in batch writer flush errors, and document the failed batch policy on WithFlushSupport
- Cover deletes before a flush, SkipInsert and stuck flushes in the conformance test, and the WriteStream exits when a plugin stops writing
- Guard flush errors against empty and invalid UTF-8 messages
- Abort checkpoints queued behind resolvers that are stuck after a cancel
- Drop checkpoints sent after SyncAll returns instead of panicking
- Send keys changed by the same scopes in one checkpoint
- Document that Write wrappers must forward every WriteFlush
- Bump plugin-pb-go to the regenerated flush_error wording
- Cover the stuck resolver, scope collapse, writer flush failures and WriteStream ending after a failed flush
- Name the tables of a failed flush, and report each write stream only the failures of its own tables
- Return the cancel cause from Sync when it drops checkpoints after the batcher closes
- Compact scope sets again only once they double, and lock the root of merged checkpoints reliably
- Describe flush results as attempts to commit, and the append mode duplicates of a failed flush
- Test the order of checkpoints from ended resolvers, scope compaction, keys with different previous checkpoints, stuck resolvers with a state client, the send guard and the flush wait
- Bump plugin-pb-go to the failed_tables field
@disq
disq marked this pull request as ready for review October 16, 2026 10:51
@disq
disq requested a review from a team as a code owner October 16, 2026 10:51
@disq
disq requested a review from marianogappa October 16, 2026 10:51
disq added 2 commits October 16, 2026 13:22
- Keep FailedTablesError usable without an underlying error
- Name the deletes a failed migration or delete-stale leaves unwritten
- Test failed tables with several failing tables in every writer, and writers.FailedTables
- Test the drops of cancelled checkpoints, merged aborts and the cost of scope compaction
- Bound the waits of test helpers, run checkpoint tests with 10 concurrent resolvers, and require the row in the ended resolver test
- Explain why a checkpoint flushes every table's batch
- Count the scopes compactions visit, call sendAfterSync directly, and run the merge race more often
- Bound the remaining test waits, and stop repeated subtests at the first failure
- Fix the comments on checkpoint batch flushes and pending deletes, and say why failures are tracked before the worker gets an insert
- Say that shuffle-queue holds checkpointed state until the sync completes
- Let a zero-value WriteFlush be used, creating its result channel on first use
- Document that a streaming writer flush reports the failures of shared workers
@disq disq added the automerge label Oct 19, 2026
@disq disq removed the automerge label Oct 19, 2026
Public API:
- writers.FailedTables becomes writers.WithFailedTables, as it wraps an error, unlike message.FailedTables
- Remove message.NewWriteFlush, as the zero WriteFlush works
- Add SkipFlush to the writer test suite
- Say the CLI, not the client, persists checkpoints
- state.NewClientWithOptions returns an error when ClientOptions sets Checkpoints, which it cannot honour

Internal:
- checkpoint.Sent becomes Delivery, with Delivered, and Send becomes Schedule
- Covering becomes Ordering, with SendsAfter
- The state client's sent map becomes last, and abortedKeys becomes requeueAborted
- The sync checkpoints use stopped, allSent, stoppedCh, loopExited, callerClosesRes and held for what they track
@disq disq added the automerge label Oct 19, 2026
@kodiakhq
kodiakhq Bot merged commit a4bba72 into main Oct 19, 2026
11 checks passed
@kodiakhq
kodiakhq Bot deleted the claude/incremental-syncs-design-08b5fc branch October 19, 2026 11:31
kodiakhq Bot pushed a commit that referenced this pull request Oct 19, 2026
🤖 I have created a release *beep* *boop*
---


## [4.99.0](v4.98.0...v4.99.0) (2026-10-19)


### Features

* Send incremental state as checkpoints after the resources it covers ([#2632](#2632)) ([a4bba72](a4bba72))


### Bug Fixes

* **deps:** Update aws-sdk-go-v2 monorepo ([#2629](#2629)) ([fae9da3](fae9da3))
* **deps:** Update aws-sdk-go-v2 monorepo ([#2635](#2635)) ([9f96272](9f96272))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.28.0 ([#2625](#2625)) ([b1a7243](b1a7243))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.28.1 ([#2627](#2627)) ([fc17c1b](fc17c1b))
* **deps:** Update module github.com/cloudquery/plugin-pb-go to v1.28.2 ([#2634](#2634)) ([1e815b0](1e815b0))
* Don't drop work queued by the last active table in shuffle-queue ([#2633](#2633)) ([223018b](223018b))
* Make the JSON type schema match the JSON that the encoder writes ([#2620](#2620)) ([befd5cc](befd5cc))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
kodiakhq Bot pushed a commit to cloudquery/plugin-sdk-python that referenced this pull request Oct 20, 2026
#### Summary

Brings the source side of state checkpoints (cloudquery/plugin-pb#42, cloudquery/plugin-sdk#2632, cloudquery/cloudquery#23546) to the Python SDK. When the CLI sets `Sync.Request.supports_checkpoints`, flushed state goes on the sync stream as `Sync.MessageCheckpoint` instead of being written straight to the backend. The CLI then persists it only after every destination commits the records synced before it.

Why we want this: some of our Python syncs run for many hours, often limited by provider rate limits. A transient failure late in the sync currently throws away all the progress, because state is only safe to write once the sync ends. With checkpoints, a failed sync keeps the cursors whose records were committed, and the next run resumes from there.

Opt-in, and unchanged unless both sides opt in:

```python
self._scheduler = Scheduler(concurrency=...)
self._state_client = StateClientBuilder.build(
    backend_options=options.backend_options,
    checkpoints=self._scheduler.checkpoints,  # new, optional
)
self._scheduler.set_post_sync_hook(self._state_client.flush)

# in a table resolver, with one cursor per form
key = state_key("responses", client_id=client.id(), form_id=form["id"])
for item in api.list_responses(form["id"], since=state_client.get_key(key)):
    yield item
    state_client.set_key(key, item["updated_at"])
    state_client.flush()  # a checkpoint, sent once this item and its relations are on the stream
```

Without `checkpoints`, or when the CLI doesn't support them, `flush()` writes to the backend as it does today.

#### Ordering

This follows the Go SDK's semantics:
- A state change made inside a table resolver is tied to that resolver call's scope. The scheduler records the scope in a thread-local while the resolver runs.
- On `flush()`, each group of keys becomes a checkpoint. It goes on the scheduler's queue once every resource those scopes produced before the flush is on it, including the relations those resources spawned.
  - Child resolvers run asynchronously on the depth pools, so the checkpoint waits for every child scope that was still open at flush time.
  - Unlike Go, the Python scheduler has no batcher: resources reach the stream in queue order, so nothing needs flushing first.
- A checkpoint also waits for earlier unsent checkpoints of the same keys. The state table has no `version` column, so the CLI relies on stream order.
- Changes made outside table resolvers wait for nothing. Flush them after the resources they summarize, for example in the post-sync hook. Checkpoints flushed by the post-sync hook are now sent before `Scheduler.sync` returns.
- Flushing in checkpoint mode while no sync is running raises an error instead of dropping the state.

#### Granularity

Two things set how much a checkpointing sync writes, and today plugin authors and users control only one of them:

- **How many keys change between commits.** That's the plugin's choice: one cursor per table, or one per parent, account or other partition. This PR adds `state_key(table, *, client_id=None, **partition)`, so plugins choose the granularity without each inventing a key format. Keys are deterministic, partition fields are sorted, and separators are escaped, so tables, clients and partitions never share a key. Each key is one state row, and a partition's checkpoint waits only for the resolver that set it. Finer keys mean a resumed sync re-fetches less, at the cost of more rows per commit.
- **How often the CLI commits.** The CLI merges checkpoints and commits at most every 30 seconds (`checkpointInterval` in `cli/cmd/sync_v3_checkpoints.go`). Each commit flushes every destination, then writes every key changed since the last commit to the state backend. The interval is hard-coded.

We'd like the commit interval to be configurable per sync, for example in the source spec next to `backend_options`, including an option to commit only at the end. The right value depends on the destination and the sync, not on the plugin:
- **Destination batches:** each flush ends the destination's batch, so a 30-second interval caps batch size by time. For S3 it also writes a new object per table every 30 seconds.
- **State backend load:** a sync with per-partition keys writes up to one row per partition per commit to a relational state backend.
- **Sync length:** for a sync that runs many hours against rate-limited APIs, committing every few minutes loses little on failure and writes far less. A short sync gains nothing from mid-sync commits.

Keeping the plugin-side flush cheap matters too, and it is: `flush()` in checkpoint mode only puts a message on the sync stream, and nothing touches a database until the CLI commits.

#### Other changes

- **State client:** now thread-safe. It clears changed keys after a successful flush (previously every flush rewrote every key changed since the start), and it keeps them after a failed one so a later flush retries them.
- **Backend options:** `BackendOptions` gains `supports_checkpoints`. The server now builds `BackendOptions` from the request instead of passing the protobuf message through.
- **Server guard:** the server refuses to send a checkpoint when the CLI didn't ask for them.
- **Key helper:** `state_key` is exported from `cloudquery.sdk.stateclient`.

#### Depends on

The `plugin-pb-python` bindings with the new messages (cloudquery/plugin-pb-python#250), which aren't released yet (release PR cloudquery/plugin-pb-python#225). `pyproject.toml` requires `>=0.0.55`, but `uv.lock` still needs updating after that release, so CI will fail until then. That's why this is a draft.

#### Open questions

1. **Failed relations.** Like Go, a parent's checkpoint still goes out when one of its child resolvers fails. For a parent cursor such as `updated_since`, that can skip the failed parent's children for good. Would you take an opt-in to hold back a scope's checkpoints when anything in its subtree errored?
2. **`version` column.** Should the Python state table get the `version` column, to match the Go client?
3. **Python destinations.** Python destination plugins don't implement `Write.MessageFlush` yet. I left that for a separate PR, since the CLI only enables checkpoints when every destination supports flushing.
4. **Full syncs.** Checkpoints only help tables that keep cursors. Resuming long full (non-incremental) syncs, and the interaction with `delete-stale`, is a bigger question. I'd like to raise it in a separate issue.

#### Testing

- New tests in `tests/scheduler/checkpoints.py`, `tests/serve/checkpoints.py` and `tests/stateclient/keys.py`. They cover:
  - parent checkpoints following all child resources of the parents they cover, with randomized child latency
  - child checkpoints
  - per-key order under concurrent flushes
  - post-sync hook checkpoints
  - backend writes without checkpoints
  - failed-flush retry
  - the builder and server gating
  - `state_key` ordering, escaping and uniqueness
- Removing either the wait on child scopes or the per-key ordering makes the parent-checkpoint test fail.
- The full suite passes locally against `plugin-pb-python` main, except `test_plugin_package`, which builds a Docker image and fails the same way locally on `main`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants